Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RSDK-8714: add Servo wrappers #60

Merged
merged 4 commits into from
Sep 11, 2024
Merged

Conversation

gloriacai01
Copy link
Member

Add servo component wrappers in java sdk + associated tests

njooma
njooma previously requested changes Aug 30, 2024
Copy link
Member

@njooma njooma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost! Seems like you forgot doCommand in a few places. But overall this is great!

responseObserver.onNext(Common.GetGeometriesResponse.newBuilder().addAllGeometries(geometries).build());
responseObserver.onCompleted();

}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is missing doCommand

verify(servo).getPosition(Struct.getDefaultInstance())
assertEquals(80, pos)
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing the test for doCommand

@lia-viam
Copy link
Contributor

lia-viam commented Sep 9, 2024

I think this needs a rebase but other than that you're probably good to merge!

Copy link
Member

@purplenicole730 purplenicole730 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For better recording purposes, please include the ticket number in the title of this PR. (It also helps with integrating with Jira)

ex: RSDK-XXXX: add servo wrappers

@gloriacai01 gloriacai01 changed the title Servo RSDK-8714: add Servo wrappers Sep 11, 2024
@lia-viam lia-viam dismissed njooma’s stale review September 11, 2024 18:40

changes addressed

@gloriacai01 gloriacai01 merged commit ecac847 into viamrobotics:main Sep 11, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants